-
Notifications
You must be signed in to change notification settings - Fork 750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move to verify #1165
move to verify #1165
Conversation
The approach seems fine to me, thank you! There's a unit test failing though because the |
yep fixed the license header |
@@ -53,7 +53,7 @@ private int ScanPath(string path, StringBuilder error) | |||
|| file.Contains("AssemblyInfo.cs") | |||
|| file.Contains(".Designer.cs") | |||
|| file.Contains(".Generated.cs") | |||
|| file.Contains(".approved.cs") | |||
|| file.Contains(".verified.cs") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, LGTM!
ok. i will add a extension point to Verify so u can intercept the files on failure. |
unfortunately in the end i had to add the following to the csproj to get this to work
The reason being is that Deterministic seems to not just clean pdb paths (which breaks approvaltests), it also cleans CallerFilePath attributes (which breaks Verify). So with that workaround in mind, it is up to you if you want to use Verify or ApprovalTests |
What's the difference between ApprovalTests or Verify? |
https://github.com/SimonCropp/Verify/blob/master/docs/compared-to-approvaltests.md disclaimer: i am a maintainer on both |
Cool, I'll stick with Verify. For now, I can set |
@clairernovotny this approach does not use pdbs to determine the path. can u verify it work for your deterministic builds?
if it does, and u are happy with this as an approach, i will finish the diffplex logging and u can merge